Skip to content

Write .b2z stores and downloads with the umask's permissions again, not 0600 - #789

Open
FabianIsensee wants to merge 1 commit into
Blosc:mainfrom
FabianIsensee:fix-b2z-file-mode
Open

FabianIsensee wants to merge 1 commit into
Blosc:mainfrom
FabianIsensee:fix-b2z-file-mode

Conversation

@FabianIsensee

Copy link
Copy Markdown
Contributor

Since 4.8.0 every .b2z that DictStore/TreeStore write or rewrite ends up with mode 0600, whatever the umask: to_b2z() builds the zip in tempfile.mkstemp(), which always creates 0o600, and os.replace()s it onto the target (3ea9619, "Make to_b2z() atomic"). On shared filesystems the group and other users can no longer read the stores, and a store the user had chmod-ed (say 0640) drops to 0600 on its next rewrite. To reproduce: umask 002, then with blosc2.TreeStore("t.b2z", mode="w") as t: t["/a"] = np.arange(3) gives -rw------- on 4.8.0, 4.14.1 and main; 4.7.0 gave -rw-rw-r--. .b2d, .b2e and .b2nd files are not affected.

The same mkstemp() + os.replace() pattern is used by RemoteStore.save() (.b2z exports), RemoteFile.download(), the b2view OrdinaryFile.download() and remote_store_cache.atomic_write() (RemoteStore disk cache files such as active_generation.json, which ended up 0600 next to 0644 arrays in the same cache directory), so their files were 0600 too. materialize() to .b2z and RemoteCTable saves to .b2z were affected through to_b2z().

This PR adds blosc2.core.mkstemp_for_replace() and uses it at those sites instead of mkstemp():

  • A new target: the temporary file is created beside it with os.open(..., O_CREAT | O_EXCL, 0o666), so the kernel applies the umask (or the directory's default ACL) exactly as for a plain open(). The umask itself is never read: os.umask() can only be read by setting it, process-wide, which would race with other threads creating files.
  • An existing target: the temporary file gets its permission bits, so a rewritten store keeps the mode it had, as when 4.7.0 truncated it in place. This also avoids widening a file the user made private. The file is created no more open than the target (target_mode & 0o666) and then set with fchmod() on the fd, so nobody can open it with wider permissions in between. The chmod is best effort: filesystems that refuse it (e.g. vfat) keep their own modes.
  • Windows is unchanged (only a read-only flag exists there).
  • to_b2z() and the RemoteStore export now write the zip through that fd instead of reopening the temporary file by path, so a read-only .b2z can still be replaced as on main (and stays read-only).
  • publish_materialized() keeps a replaced .b2z's mode too.
  • The write stays atomic.

Files written 0600 by 4.8.0-4.14.x keep that mode when they are rewritten; a chmod fixes them once.

Results, umask 022 / 002 (main vs this PR; for every .b2z row, 4.7.0 gave the same as this PR):

  • new .b2z (DictStore/TreeStore close, to_b2z(), materialize(), RemoteStore.save()), downloads, cache files: 0600 / 0600 → 0644 / 0664
  • rewriting a 0640 .b2z (mode "a" or "w", to_b2z(overwrite=True), materialize(overwrite=True), save(overwrite=True)) or a 0640 download target: 0600 → 0640
  • directory with a default ACL (g::rwx, o::---), umask 022: open() gives 0660, main 0600, this PR 0660

Tests: tests/test_file_mode.py (umask 022 and 002; skipped on Windows and WASM) covers:

  • new and rewritten .b2z stores (modes "w" and "a"), to_b2z() and materialize();
  • RemoteStore.save() plus its disk cache files;
  • OrdinaryFile.download() and RemoteFile.download();
  • the mode of the temporary file at creation;
  • the rewrite of a read-only .b2z.

15 of the 17 fail on main; the other 2 check the temporary file's mode at creation, which mkstemp's 0600 always passes. Full suite: same results as main apart from the new tests. No performance impact: one stat per write, plus a chmod when the target exists.

I used Claude Code (Anthropic) to find the affected writers, write the fix and tests, and draft this description; I reviewed the change.

🤖 Generated with Claude Code

Since to_b2z() became atomic (3ea9619, 4.8.0) it builds the zip in
tempfile.mkstemp() and os.replace()s it onto the target.  mkstemp()
always creates 0o600, so every .b2z written or rewritten since then is
private to its owner whatever the umask, and a file the user had
chmod-ed loses its mode.  On shared filesystems the group can no longer
read the stores.  RemoteStore exports, RemoteFile/OrdinaryFile
downloads and the RemoteStore disk cache's manifests use the same
pattern and have the same problem.

Add core.mkstemp_for_replace(): it creates the temporary file beside
the target with os.open(..., O_CREAT | O_EXCL, 0o666), so the kernel
applies the umask (or the directory's default ACL) exactly as for a
plain open(); the umask is never read, since os.umask() can only be
read by setting it process-wide, which races with other threads.  If
the target exists, the temporary file is created no more open than it
and then gets its permission bits (fchmod), as an in-place write would
have kept them (4.7.0 behaviour).  Windows is unchanged.  Use it at
every mkstemp() + os.replace() site; to_b2z() and the RemoteStore
export now write the zip through the fd, so a read-only target can
still be replaced, as on main.  publish_materialized() keeps a replaced
.b2z's mode too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SwMZj6jcia1iL27SPfQCoh

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant